Add OIDC provider functionality with validation setup#548
Add OIDC provider functionality with validation setup#548shreknel wants to merge 13 commits intotinyauthapp:mainfrom
Conversation
This commit adds OpenID Connect (OIDC) provider functionality to tinyauth, allowing it to act as an OIDC identity provider for other applications. Features: - OIDC discovery endpoint at /.well-known/openid-configuration - Authorization endpoint for OAuth 2.0 authorization code flow - Token endpoint for exchanging authorization codes for tokens - ID token generation with JWT signing - JWKS endpoint for public key distribution - Support for PKCE (code challenge/verifier) - Nonce validation for ID tokens - Configurable OIDC clients with redirect URIs, scopes, and grant types Validation: - Docker Compose setup for local testing - OIDC test client (oidc-whoami) with session management - Nginx reverse proxy configuration - DNS server (dnsmasq) for custom domain resolution - Chrome launch script for easy testing Configuration: - OIDC configuration in config.yaml - Example configuration in config.example.yaml - Database migrations for OIDC client storage
📝 WalkthroughWalkthroughAdds an in-process OpenID Connect provider: new OIDC config and example, DB migrations and GORM models for clients/keys/authorization-codes, an OIDCService (RSA keys, JWTs, PKCE, client sync), an OIDCController (discovery/authorize/token/userinfo/jwks), bootstrap wiring, dependency, and validation tooling. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (User)
participant Client as OIDC Client (App)
participant Controller as OIDCController
participant Auth as AuthService
participant OIDC as OIDCService
participant DB as Database
rect rgb(200,220,250)
Note over Browser,DB: Authorization Code Flow (authorize)
end
Browser->>Controller: GET /oidc/authorize?client_id=...&redirect_uri=...&response_type=code&state=...
Controller->>OIDC: GetClient(client_id)
OIDC->>DB: SELECT oidc_clients WHERE client_id=...
DB-->>OIDC: client record
OIDC-->>Controller: client config
Controller->>OIDC: ValidateRedirectURI/ResponseType/Scope
Controller->>Auth: Check authentication
alt not authenticated
Controller-->>Browser: Redirect /login (preserve original request)
else authenticated
Auth-->>Controller: userContext
Controller->>OIDC: GenerateAuthorizationCode(userContext,...,pkce)
OIDC-->>Controller: code (JWT)
Controller-->>Browser: Redirect to redirect_uri?code=...&state=...
end
sequenceDiagram
participant Client as OIDC Client (Confidential)
participant Controller as OIDCController
participant OIDC as OIDCService
participant DB as Database
rect rgb(220,250,200)
Note over Client,DB: Token Exchange Flow (token)
end
Client->>Controller: POST /oidc/token (grant_type=authorization_code, code, redirect_uri, client creds)
Controller->>Controller: Extract client creds (BasicAuth or form)
Controller->>OIDC: GetClient(client_id)
OIDC->>DB: SELECT oidc_clients WHERE client_id=...
DB-->>OIDC: client record
Controller->>OIDC: VerifyClientSecret(...)
Controller->>OIDC: ValidateAuthorizationCode(code, client_id, redirect_uri)
OIDC-->>Controller: userContext, scopes, nonce
Controller->>OIDC: GenerateAccessToken(userContext, client_id, scopes)
OIDC-->>Controller: access_token (JWT)
Controller->>OIDC: GenerateIDToken(userContext, client_id, nonce)
OIDC-->>Controller: id_token (JWT)
Controller-->>Client: JSON { access_token, id_token, token_type, expires_in, scope }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (9)
internal/config/config.go (2)
72-88: Consider enforcing default values for OIDC configuration.The struct field descriptions specify defaults (e.g.,
grantTypesdefaults to['authorization_code'],scopesdefaults to['openid', 'profile', 'email']), but these are not enforced in code. Consider adding validation or initialization logic to apply these defaults when fields are empty, ensuring consistent behavior.Example approach for enforcing defaults
You could add a method to normalize/validate the config after loading:
func (c *OIDCClientConfig) ApplyDefaults() { if len(c.GrantTypes) == 0 { c.GrantTypes = []string{"authorization_code"} } if len(c.ResponseTypes) == 0 { c.ResponseTypes = []string{"code"} } if len(c.Scopes) == 0 { c.Scopes = []string{"openid", "profile", "email"} } }Call this method after loading the configuration from YAML.
80-82: Document the mutual exclusivity ofClientSecretandClientSecretFile.Both
ClientSecretandClientSecretFilefields are present. Based on the pattern inOAuthServiceConfig, these appear to be mutually exclusive. Consider adding a comment or validation to clarify that only one should be set.validation/Dockerfile (1)
5-5: Consider pinning dependency versions for reproducibility.The
pip installcommand installsrequestsandauthlibwithout version constraints. For reproducible builds and to avoid unexpected breakage, consider pinning specific versions.Example with pinned versions
-RUN pip install --no-cache-dir requests authlib +RUN pip install --no-cache-dir requests==2.31.0 authlib==1.3.0Note: Update to current stable versions as appropriate.
internal/assets/migrations/000004_oidc_clients.up.sql (1)
2-2: Remove redundant UNIQUE constraint on PRIMARY KEY.The
client_idcolumn is declared as bothPRIMARY KEYandUNIQUE. ThePRIMARY KEYconstraint already enforces uniqueness, making theUNIQUEkeyword redundant.Suggested fix
- "client_id" TEXT NOT NULL PRIMARY KEY UNIQUE, + "client_id" TEXT NOT NULL PRIMARY KEY,validation/launch-chrome.sh (1)
22-38: Consider verifying executable status.The Chrome detection logic covers common paths well. However, line 34 checks file existence with
[ -f "$path" ]but doesn't verify if the file is executable. In practice, Chrome binaries should be executable, but for robustness, consider:🔎 Optional improvement for executable check
CHROME_CMD="" for path in "${CHROME_PATHS[@]}"; do - if [ -f "$path" ] || command -v "$(basename "$path")" &> /dev/null; then + if [ -x "$path" ] || command -v "$(basename "$path")" &> /dev/null; then CHROME_CMD="$(basename "$path")" break fi doneThis is validation tooling, so the current implementation is acceptable.
internal/model/oidc_client_model.go (1)
3-13: Consider adding helper methods for JSON array fields.The model structure is appropriate, but the JSON array fields (lines 7-10) stored as strings will require careful marshaling/unmarshaling in the service layer.
To improve type safety and reduce marshaling errors, consider adding helper methods:
🔎 Optional helper methods
// GetRedirectURIsSlice unmarshals the RedirectURIs JSON string into a slice func (c *OIDCClient) GetRedirectURIsSlice() ([]string, error) { var uris []string if err := json.Unmarshal([]byte(c.RedirectURIs), &uris); err != nil { return nil, err } return uris, nil } // SetRedirectURIsSlice marshals a slice into the RedirectURIs JSON string func (c *OIDCClient) SetRedirectURIsSlice(uris []string) error { data, err := json.Unmarshal(uris) if err != nil { return err } c.RedirectURIs = string(data) return nil } // Similar methods for GrantTypes, ResponseTypes, and Scopes...This encapsulates the JSON handling logic within the model, reducing the chance of marshaling errors elsewhere.
validation/README.md (1)
22-22: Consider addressing markdown linting suggestions.Static analysis identified several minor markdown formatting issues that could improve documentation consistency:
- Bare URLs (lines 22, 30, 32, 38, 66, 88, 135, 163, 165): Consider wrapping in angle brackets or markdown links
- Missing language specifier (line 84): The
/etc/hostscode block should specify a language (e.g.,textorhosts)- Space in code span (line 132): Remove spaces inside the backticks in
--host-resolver-rules=...Example fixes:
🔎 Proposed formatting improvements
For bare URLs:
-- **Access:** http://auth.example.com/ (via nginx on port 80) +- **Access:** <http://auth.example.com/> (via nginx on port 80)For code block language:
-``` +```text 127.0.0.1 auth.example.comFor code span spacing: ```diff - - In "Target" field, append: ` --host-resolver-rules="MAP auth.example.com 127.0.0.1"` + - In "Target" field, append: `--host-resolver-rules="MAP auth.example.com 127.0.0.1"`Based on static analysis hints. These are purely cosmetic improvements.
Also applies to: 30-30, 32-32, 38-38, 66-66, 84-86, 88-88, 132-132, 135-135, 163-163, 165-165
validation/oidc_whoami.py (1)
263-267: Consider adding SameSite attribute to session cookie.The
id_tokencookie is set withHttpOnlybut lacksSameSiteattribute. For better CSRF protection, consider addingSameSite=Lax. Since this is a test client over HTTP, theSecureflag would break functionality.🔎 Proposed fix
- self.send_header("Set-Cookie", f"id_token={token['id_token']}; Path=/; Max-Age={max_age}; HttpOnly") + self.send_header("Set-Cookie", f"id_token={token['id_token']}; Path=/; Max-Age={max_age}; HttpOnly; SameSite=Lax")internal/service/oidc_service.go (1)
332-343: JWKS exponent encoding may include unnecessary leading zeros.The exponent is always encoded as 4 bytes, but the JWK spec requires minimal encoding (no leading zeros). The common RSA exponent 65537 (
0x010001) should be encoded as 3 bytes, not 4.🔎 Proposed fix: Strip leading zero bytes
- eBytes := make([]byte, 4) - eBytes[0] = byte(e >> 24) - eBytes[1] = byte(e >> 16) - eBytes[2] = byte(e >> 8) - eBytes[3] = byte(e) + // Encode exponent with minimal bytes (strip leading zeros) + eBytes := make([]byte, 0, 4) + for shift := 24; shift >= 0; shift -= 8 { + b := byte(e >> shift) + if len(eBytes) > 0 || b != 0 { + eBytes = append(eBytes, b) + } + } + if len(eBytes) == 0 { + eBytes = []byte{0} + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (20)
config.example.yamlgo.modinternal/assets/migrations/000004_oidc_clients.down.sqlinternal/assets/migrations/000004_oidc_clients.up.sqlinternal/bootstrap/router_bootstrap.gointernal/bootstrap/service_bootstrap.gointernal/config/config.gointernal/controller/oidc_controller.gointernal/model/oidc_client_model.gointernal/service/oidc_service.gointernal/utils/app_utils.gointernal/utils/loaders/loader_file.govalidation/Dockerfilevalidation/README.mdvalidation/config.yamlvalidation/docker-compose.ymlvalidation/launch-chrome-host.shvalidation/launch-chrome.shvalidation/nginx.confvalidation/oidc_whoami.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T11:02:49.689Z
Learnt from: steveiliop56
Repo: steveiliop56/tinyauth PR: 326
File: internal/service/database_service.go:7-11
Timestamp: 2025-08-27T11:02:49.689Z
Learning: In internal/service/database_service.go, using glebarez/sqlite (modernc) for GORM with sqlite3 (mattn) driver for golang-migrate is a valid pattern that works correctly, despite appearing to be a driver mismatch. The golang-migrate sqlite driver causes panics when used with modernc.org/sqlite backend, so the mixed approach should be maintained.
Applied to files:
go.mod
🧬 Code graph analysis (2)
internal/bootstrap/router_bootstrap.go (1)
internal/controller/oidc_controller.go (2)
NewOIDCController(31-38)OIDCControllerConfig(19-22)
internal/bootstrap/service_bootstrap.go (1)
internal/service/oidc_service.go (3)
OIDCService(33-37)NewOIDCService(39-43)OIDCServiceConfig(25-31)
🪛 markdownlint-cli2 (0.18.1)
validation/README.md
22-22: Bare URL used
(MD034, no-bare-urls)
30-30: Bare URL used
(MD034, no-bare-urls)
32-32: Bare URL used
(MD034, no-bare-urls)
38-38: Bare URL used
(MD034, no-bare-urls)
66-66: Bare URL used
(MD034, no-bare-urls)
84-84: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
88-88: Bare URL used
(MD034, no-bare-urls)
132-132: Spaces inside code span elements
(MD038, no-space-in-code)
135-135: Bare URL used
(MD034, no-bare-urls)
163-163: Bare URL used
(MD034, no-bare-urls)
165-165: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.10)
validation/oidc_whoami.py
42-42: Avoid specifying long messages outside the exception class
(TRY003)
72-72: Probable use of requests call without timeout
(S113)
99-99: Do not catch blind exception: Exception
(BLE001)
293-293: Do not catch blind exception: Exception
(BLE001)
297-297: Possible binding to all interfaces
(S104)
🪛 Shellcheck (0.11.0)
validation/launch-chrome-host.sh
[warning] 31-31: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (31)
internal/utils/app_utils.go (1)
5-5: LGTM! Enhanced error messages improve debuggability.The addition of contextual information (URLs, IP addresses, part counts) to error messages using
fmt.Errorfis a good practice that will significantly help troubleshooting configuration issues without changing the validation logic.Also applies to: 26-26, 32-32, 40-40
config.example.yaml (1)
66-100: Well-documented OIDC configuration example.The OIDC configuration block is clearly documented with helpful comments and aligns well with the struct definitions in
internal/config/config.go. The placeholder values make it easy for users to understand what needs to be configured.internal/assets/migrations/000004_oidc_clients.down.sql (1)
1-1: LGTM!The down migration correctly uses
DROP TABLE IF EXISTSfor safe idempotent rollback.internal/bootstrap/router_bootstrap.go (1)
105-113: Good defensive initialization pattern.The conditional OIDC controller setup properly checks both the config flag (
app.config.OIDC.Enabled) and service availability (app.services.oidcService != nil), preventing initialization errors if the service failed to start.validation/config.yaml (1)
1-36: Validation configuration looks appropriate for local testing.The configuration correctly sets
secureCookie: falseand uses HTTP URLs, which is appropriate for local Docker-based testing. The test credentials and client secrets are clearly for development/validation purposes.go.mod (1)
14-14: No action required — v5.3.0 is the current, latest stable release with all known security patches applied.The version
github.com/golang-jwt/jwt/v5 v5.3.0(released July 30, 2025) is the latest stable release. It includes fixes for CVE-2025-30204 (DoS via excessive memory allocation in parse.ParseUnverified) and CVE-2024-51744 (error handling in ParseWithClaims), which were patched in v5.2.2 and later. No further upgrades are needed.internal/assets/migrations/000004_oidc_clients.up.sql (1)
5-8: Array fields are properly serialized as JSON with error handling in place.All four columns (
redirect_uris,grant_types,response_types,scopes) are correctly stored as JSON arrays. The OIDCClient model includes// JSON arraydocumentation, and the OIDCService implements proper serialization withjson.Marshal()when writing to the database and deserialization withjson.Unmarshal()when reading, complete with error logging for both operations.internal/utils/loaders/loader_file.go (2)
28-37: LGTM! Good error handling and logging.The empty path check, contextual logging, and error handling are all appropriate. Including the config file path in both warning and error messages aids troubleshooting.
19-26: Code looks good. The dual-check pattern for flag keys (traefik.experimental.configfileandexperimental.configfile) appropriately handles both namespaced and direct flag formats. The comment about paerser converting flags to lowercase is accurate—the flag keys checked are consistently lowercase, matching the paerser library's normalization behavior.validation/nginx.conf (3)
1-9: LGTM! Appropriate Docker DNS configuration.The resolver configuration correctly uses Docker's built-in DNS (127.0.0.11) for service name resolution. The 10-second cache validity and 5-second timeout are reasonable for a test environment.
11-25: LGTM! Good use of dynamic backend resolution.The dynamic backend variable pattern enables nginx to resolve the backend at request time, which is appropriate for Docker service discovery. All standard proxy headers are properly forwarded.
27-41: LGTM! Consistent configuration pattern.The client.example.com server block follows the same reliable pattern as auth.example.com, ensuring consistent proxy behavior across both services.
internal/bootstrap/service_bootstrap.go (2)
16-16: LGTM! Field addition follows existing pattern.The oidcService field addition is consistent with other service fields in the struct.
100-132: LGTM! Well-structured graceful degradation pattern.The OIDC service initialization follows a robust pattern:
- Conditional initialization gated by
Enabledflag- Sensible default for issuer (AppURL)
- Non-fatal error handling with appropriate logging
- Client sync attempts with success/failure logging
This pattern is consistent with the LDAP service initialization approach (lines 43-49), ensuring optional features don't break bootstrap.
validation/launch-chrome.sh (3)
8-20: LGTM! Appropriate service readiness check.The 30-second timeout with polling is a good balance for waiting on nginx startup in a container environment.
40-51: LGTM! User-friendly fallback handling.The manual instructions fallback is well-structured and exits gracefully with status 0, which is appropriate since Chrome is expected to be on the host, not in the container.
57-67: LGTM! Appropriate Chrome configuration for testing.The host-resolver-rules approach is ideal for testing, avoiding system-wide DNS changes. The background execution and helpful output URLs provide a good user experience.
validation/launch-chrome-host.sh (3)
9-21: LGTM! Appropriate host-side Chrome detection.The Chrome detection logic properly errors when Chrome isn't found, which is appropriate for a host-side script where Chrome installation is expected.
23-26: LGTM! Clear user feedback.The pre-launch output provides helpful context about which Chrome binary is being used and the target URL.
36-39: LGTM! Helpful post-launch output.The success message and test URLs provide clear next steps for the user.
validation/docker-compose.yml (4)
4-21: LGTM! Well-configured DNS service.The dnsmasq configuration appropriately maps test domains to the nginx proxy IP and uses Google's public DNS as upstream resolvers. The NET_ADMIN capability is required for dnsmasq operations.
23-38: LGTM! Appropriate reverse proxy setup.The nginx service correctly depends on all upstream services and properly exposes port 80 for host access. The comments clarify the DNS resolution strategy.
40-59: LGTM! Proper OIDC provider configuration.The tinyauth service is well-configured with:
- Experimental config file flag (consistent with loader_file.go changes)
- Appropriate volume mounts for data persistence and config
- Healthcheck for service readiness
- Internal networking without host port exposure
85-90: LGTM! Appropriate network configuration.The custom bridge network with a /16 subnet provides ample IP space for the test environment and allows static IP assignments for the services.
internal/model/oidc_client_model.go (1)
15-17: LGTM! Standard GORM table name implementation.The TableName method correctly specifies the database table name following GORM conventions.
validation/README.md (1)
1-181: Excellent comprehensive documentation!The validation README provides thorough documentation covering:
- Step-by-step setup instructions
- Multiple browser access methods (hosts file, Chrome flags, system DNS)
- Platform-specific guidance (Linux, macOS, Windows)
- Clear configuration details
- Helpful notes about networking and cookie domains
The documentation will significantly aid developers and testers in working with the OIDC setup.
validation/oidc_whoami.py (1)
44-61: Single-session limitation: state and nonce are shared globally.The
stateandnonceare generated once at module startup and shared across all requests. This means only one concurrent OIDC flow is supported. This is acceptable for a single-user test client but worth noting.internal/controller/oidc_controller.go (2)
52-72: LGTM: Discovery document is well-formed.The OpenID Connect discovery document includes all required fields and correctly advertises the supported capabilities including PKCE support.
171-274: Token handler structure is correct.The token endpoint properly implements the authorization code flow: validates grant type, extracts credentials, verifies client, validates authorization code, and generates tokens. Error handling follows OAuth 2.0 error response format.
internal/service/oidc_service.go (2)
365-467: Client sync logic is well-implemented.The
SyncClientsFromConfigmethod properly handles both creating new clients and updating existing ones, with sensible defaults for missing configuration values and comprehensive error logging.
471-504: LGTM: Helper functions are clean and correct.The helper functions (
splitScopes,joinScopes,contains,getStringClaim) are simple, focused, and correctly implemented.
…idation The validateAccessToken method was only decoding the JWT payload without verifying the signature, allowing attackers to forge tokens. This fix: - Adds ValidateAccessToken method to OIDCService that properly verifies JWT signature using RSA public key - Validates issuer, expiration, and required claims - Updates controller to use the secure validation method - Removes insecure manual JWT parsing code
PKCE was advertised in the discovery document but not actually implemented. This commit adds full PKCE support: - Store code_challenge and code_challenge_method in authorization code JWT - Accept code_verifier parameter in token endpoint - Validate code_verifier against stored code_challenge - Support both S256 (SHA256) and plain code challenge methods - PKCE validation is required when code_challenge is present This prevents authorization code interception attacks by requiring the client to prove possession of the code_verifier that was used to generate the code_challenge.
Per OAuth 2.0 RFC 6749 §4.1.2.1, errors should NOT redirect to unvalidated redirect_uri values. This fix: - Returns JSON errors for failures before redirect_uri validation (missing parameters, invalid client) - Only redirects to redirect_uri after it has been validated against registered client URIs - Prevents open redirect attacks where malicious redirect_uri values could be used to redirect users to attacker-controlled sites
The discovery document only advertises client_secret_basic and client_secret_post as supported authentication methods. Query parameters are insecure because they are: - Logged in access logs - Stored in browser history - Exposed in referrer headers This fix removes the query parameter fallback, ensuring client secrets are only accepted via: - Authorization header (client_secret_basic) - POST form body (client_secret_post) This aligns the implementation with the advertised capabilities and prevents client secret exposure through query strings.
User claims from ID tokens (username, name, email) were directly interpolated into HTML without escaping, allowing XSS attacks if malicious content was present in claims. This fix: - Imports html module for escaping - Escapes all user-controlled data before rendering in HTML - Escapes JSON output in pre tags as well - Prevents execution of malicious scripts in browser
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
internal/service/oidc_service.go (5)
46-58: RSA keys are ephemeral: all tokens invalidate on restart.This issue was previously flagged and remains unresolved. A new RSA key pair is generated on every service initialization, invalidating all existing access tokens, ID tokens, and authorization codes when the service restarts.
72-74: Client secrets are stored and compared in plaintext.This issue was previously flagged and remains unresolved. Client secrets should be hashed (e.g., bcrypt) for storage and compared using constant-time comparison to prevent timing attacks.
142-153: Logic flaw: "openid" scope added even if not allowed for client.This issue was previously flagged and remains unresolved. If
openidis requested but not in the client'sallowedScopes, it's still added tovalidScopes, bypassing the client's configured scope restrictions.
163-171: Remove dead code:authCodemap is never used.This issue was previously flagged and remains unresolved. The
authCodemap is constructed but only suppressed with_ = authCode. Remove this dead code.Also applies to: 208-208
421-424: Remove unusedpubKeyPEMvariable.This issue was previously flagged and remains unresolved. The PEM-encoded public key is computed but never used. Remove this dead code.
Also applies to: 446-446
🧹 Nitpick comments (3)
internal/service/oidc_service.go (3)
272-301: Consider constant-time comparison for PKCE validation.Lines 288 and 293 use standard string equality (
!=) to compare the code challenge/verifier. While PKCE is primarily designed to prevent MITM attacks rather than timing attacks, usingsubtle.ConstantTimeCompareis a defense-in-depth best practice to prevent potential timing side-channels.🔎 Proposed fix using constant-time comparison
+import "crypto/subtle" + func (oidc *OIDCService) ValidatePKCE(codeChallenge string, codeChallengeMethod string, codeVerifier string) error { // ... existing checks ... switch codeChallengeMethod { case "S256": hash := sha256.Sum256([]byte(codeVerifier)) computedChallenge := base64.RawURLEncoding.EncodeToString(hash[:]) - if computedChallenge != codeChallenge { + if subtle.ConstantTimeCompare([]byte(computedChallenge), []byte(codeChallenge)) != 1 { return errors.New("code_verifier does not match code_challenge") } case "plain": - if codeVerifier != codeChallenge { + if subtle.ConstantTimeCompare([]byte(codeVerifier), []byte(codeChallenge)) != 1 { return errors.New("code_verifier does not match code_challenge") }
303-327: Add key ID (kid) to JWT headers for JWKS compatibility.The generated JWTs (access tokens and ID tokens) do not explicitly set a
kid(key ID) header, but the JWKS endpoint returns a key withkid: "default". Without thekidheader in tokens, clients cannot reliably match tokens to the correct public key, especially when key rotation is implemented in the future.🔎 Proposed fix: Set kid header when creating tokens
func (oidc *OIDCService) GenerateAccessToken(userContext *config.UserContext, clientID string, scopes []string) (string, error) { // ... existing claims setup ... - token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + token.Header["kid"] = "default" accessToken, err := token.SignedString(oidc.privateKey)Apply the same change to
GenerateIDToken(line 406) andGenerateAuthorizationCode(line 202).Also applies to: 383-413
431-435: Consider using minimum-octet encoding for RSA exponent in JWK.The RSA exponent is manually encoded as a fixed 4-byte big-endian value. RFC 7518 Section 6.3.1 recommends using the minimum number of octets needed to represent the value. For the standard exponent 65537 (0x010001), this would be 3 bytes.
While most OIDC clients tolerate the extra leading zero byte, using a standards-compliant approach improves interoperability.
🔎 Proposed fix: Use math/big for minimum-octet encoding
// Extract modulus and exponent from public key n := oidc.publicKey.N e := oidc.publicKey.E - nBytes := n.Bytes() - eBytes := make([]byte, 4) - eBytes[0] = byte(e >> 24) - eBytes[1] = byte(e >> 16) - eBytes[2] = byte(e >> 8) - eBytes[3] = byte(e) + nBytes := n.Bytes() + eBig := big.NewInt(int64(e)) + eBytes := eBig.Bytes()This automatically uses the minimum number of bytes needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/controller/oidc_controller.gointernal/service/oidc_service.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/controller/oidc_controller.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/service/oidc_service.go (4)
internal/model/oidc_client_model.go (2)
OIDCClient(3-13)OIDCClient(15-17)internal/config/config.go (1)
UserContext(151-163)internal/controller/context_controller.go (1)
Provider(40-44)internal/utils/security_utils.go (1)
GetSecret(13-28)
The variable 'html' was being assigned to store HTML content, which caused Python to treat 'html' as a local variable throughout the function. This prevented access to the 'html' module (imported at the top) within f-strings that referenced html.escape(). Renamed the HTML content variable to 'html_content' to avoid the naming conflict with the html module.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
validation/oidc_whoami.py (1)
73-73: Add timeout to JWKS fetch request (duplicate concern).The
requests.get()call lacks a timeout parameter, which could cause the client to hang if the JWKS endpoint becomes unresponsive. This issue was previously flagged.
🧹 Nitpick comments (2)
validation/oidc_whoami.py (1)
104-277: LGTM: Well-implemented OIDC callback handler.The request handler correctly implements the OIDC flow with proper security measures:
- State validation to prevent CSRF attacks (line 223)
- PKCE code verifier for token exchange (line 235)
- Claims validation with issuer and audience checks (lines 243-246)
- HttpOnly cookie flag prevents JavaScript access (line 267)
- HTML escaping prevents XSS (lines 173-175, 179)
- Cookie expiration aligned with token expiry (lines 261-262)
💡 Optional: Consider additional cookie security flags
For enhanced security (though less critical in a test client), consider adding
SameSiteandSecureflags to the cookie:- self.send_header("Set-Cookie", f"id_token={token['id_token']}; Path=/; Max-Age={max_age}; HttpOnly") + self.send_header("Set-Cookie", f"id_token={token['id_token']}; Path=/; Max-Age={max_age}; HttpOnly; SameSite=Lax")Note:
Secureflag would require HTTPS, which may not be practical for local testing.internal/model/oidc_key_model.go (1)
6-7: Consider adding GORM auto-timestamp tags.The
CreatedAtandUpdatedAtfields would benefit from GORM's automatic timestamp management to ensure consistency.🔎 Proposed enhancement
- CreatedAt int64 `gorm:"column:created_at"` - UpdatedAt int64 `gorm:"column:updated_at"` + CreatedAt int64 `gorm:"column:created_at;autoCreateTime"` + UpdatedAt int64 `gorm:"column:updated_at;autoUpdateTime"`
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
internal/assets/migrations/000005_oidc_keys.down.sqlinternal/assets/migrations/000005_oidc_keys.up.sqlinternal/model/oidc_key_model.gointernal/service/oidc_service.govalidation/oidc_whoami.py
✅ Files skipped from review due to trivial changes (1)
- internal/assets/migrations/000005_oidc_keys.up.sql
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-30T15:03:09.620Z
Learnt from: steveiliop56
Repo: steveiliop56/tinyauth PR: 122
File: internal/handlers/handlers.go:75-81
Timestamp: 2025-04-30T15:03:09.620Z
Learning: Values from identity providers and user inputs should be sanitized before being set as HTTP headers in TinyAuth to prevent CRLF injection attacks, even if they come from seemingly trusted sources like OAuth providers.
Applied to files:
validation/oidc_whoami.py
🧬 Code graph analysis (2)
internal/model/oidc_key_model.go (1)
internal/model/session_model.go (1)
UUID(3-13)
internal/service/oidc_service.go (5)
internal/model/oidc_key_model.go (2)
OIDCKey(3-8)OIDCKey(10-12)internal/model/oidc_client_model.go (2)
OIDCClient(3-13)OIDCClient(15-17)internal/config/config.go (3)
UserContext(151-163)Claims(100-106)OIDCClientConfig(80-88)internal/controller/context_controller.go (1)
Provider(40-44)internal/utils/security_utils.go (1)
GetSecret(13-28)
🪛 Ruff (0.14.10)
validation/oidc_whoami.py
43-43: Avoid specifying long messages outside the exception class
(TRY003)
73-73: Probable use of requests call without timeout
(S113)
100-100: Do not catch blind exception: Exception
(BLE001)
294-294: Do not catch blind exception: Exception
(BLE001)
298-298: Possible binding to all interfaces
(S104)
🔇 Additional comments (9)
validation/oidc_whoami.py (3)
18-24: LGTM: Clean environment-based configuration.The configuration setup correctly loads OIDC parameters from environment variables and appropriately marks
CLIENT_SECRETas optional to support public clients.
45-63: LGTM: Proper PKCE and OIDC flow setup.The authorization flow correctly implements PKCE with S256 challenge method and securely generates state and nonce parameters using
secrets.token_urlsafe.
278-298: LGTM: Server startup is appropriate for containerized testing.The server startup logic is well-suited for the validation environment:
- Clear usage instructions for manual testing
- Graceful handling of browser auto-open failures (lines 292-296)
- Binding to
0.0.0.0:8765is necessary and correct for Docker containerization, allowing the test client to be accessed from outside the container via the nginx reverse proxyNote: The static analysis warning about binding to all interfaces (S104) is a false positive in this validation/testing context.
internal/assets/migrations/000005_oidc_keys.down.sql (1)
1-1: LGTM!The down-migration correctly drops the
oidc_keystable with proper IF EXISTS guard.internal/service/oidc_service.go (5)
331-360: LGTM!The PKCE validation correctly implements both S256 (SHA256 + base64url) and plain methods per RFC 7636.
362-472: LGTM!The token generation and validation methods correctly implement OAuth 2.0 and OIDC standards, with proper expiry handling, issuer validation, and nonce support.
485-509: LGTM!The JWK generation correctly extracts the RSA modulus and exponent, encodes them as base64url, and constructs a proper JWK structure for RS256.
523-625: LGTM!The client sync logic correctly handles defaults, JSON serialization for array fields, and performs proper upsert operations while preserving creation timestamps.
627-662: LGTM!The helper functions are well-implemented with proper error handling and defensive programming (whitespace trimming, empty checks).
Authorization codes were implemented as stateless JWTs with no tracking, allowing the same code to be exchanged for tokens multiple times. This violates OAuth 2.0 RFC 6749 Section 4.1.2 which mandates that authorization codes MUST be single-use. This change: - Adds oidc_authorization_codes table to track code usage - Stores authorization codes in database when generated - Validates code exists and hasn't been used before exchange - Marks code as used immediately after validation - Prevents replay attacks where intercepted codes could be reused Security impact: - Prevents attackers from reusing intercepted authorization codes - Ensures compliance with OAuth 2.0 security requirements - Adds database-backed single-use enforcement
Security improvements: 1. Client secret hashing: - Replace plaintext comparison with bcrypt.CompareHashAndPassword - Provides constant-time comparison to prevent timing attacks - Hash secrets with bcrypt before storing in database - Update SyncClientsFromConfig to hash incoming plaintext secrets 2. Deterministic RSA key loading: - Load most recently created key using ORDER BY created_at DESC - Add warning if multiple keys detected in database - Ensures consistent key selection on startup 3. Optional RSA key encryption: - Encrypt private keys with AES-256-GCM when OIDC_RSA_MASTER_KEY is set - Master key derived via SHA256 from environment variable - Backward compatible: stores plaintext if no master key set - Automatic detection of encrypted vs plaintext on load All changes maintain backward compatibility with existing deployments.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/service/oidc_service.go (1)
316-327: Logic flaw: "openid" scope bypass still present.The special case for "openid" (lines 316-327) still allows adding "openid" to
validScopeseven when it's not in the client'sallowedScopes. This bypasses the client's configured scope restrictions. If "openid" should always be available, add it to the default scopes inSyncClientsFromConfig(line 700); otherwise, enforce the client's allowed scopes consistently.🔎 Proposed fix: Remove special case
- // Always include "openid" if it was requested - hasOpenID := false - for _, scope := range validScopes { - if scope == "openid" { - hasOpenID = true - break - } - } - - if !hasOpenID && contains(requestedScopesList, "openid") { - validScopes = append(validScopes, "openid") - } - return validScopes, nilIf "openid" must always be allowed, add it to default scopes at line 700 instead.
Based on learnings, past reviews flagged this same issue.
🧹 Nitpick comments (2)
internal/model/oidc_authorization_code_model.go (1)
7-7: Consider clarifying the GORMdefaulttag usage.The
default:falsetag in the GORM struct doesn't set the SQLDEFAULTconstraint; that's defined in the migration file (line 5 of000006_oidc_authorization_codes.up.sql). The GORM tag primarily affects behavior when creating records programmatically. Since the migration already setsDEFAULT 0, this works correctly, but the tag could be removed to avoid confusion about where defaults are actually enforced.🔎 Optional: Remove redundant default tag
- Used bool `gorm:"column:used;default:false"` + Used bool `gorm:"column:used"`This makes it clearer that the SQL default is the source of truth.
internal/service/oidc_service.go (1)
128-130: Consider enforcing minimum key length for production security.The code warns when
OIDC_RSA_MASTER_KEYis shorter than 32 bytes but continues execution. A weak master key significantly reduces the security of encrypted RSA private keys. For production deployments, consider rejecting keys that don't meet minimum entropy requirements.🔎 Proposed fix to enforce minimum key length
if len(oidc.masterKey) < 32 { - log.Warn().Msg("OIDC_RSA_MASTER_KEY is shorter than 32 bytes, consider using a longer key for better security") + return fmt.Errorf("OIDC_RSA_MASTER_KEY must be at least 32 bytes for production use (got %d bytes)", len(oidc.masterKey)) }Alternatively, keep the warning for flexibility but document the security implications clearly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/assets/migrations/000006_oidc_authorization_codes.down.sqlinternal/assets/migrations/000006_oidc_authorization_codes.up.sqlinternal/model/oidc_authorization_code_model.gointernal/service/oidc_service.go
🔇 Additional comments (8)
internal/assets/migrations/000006_oidc_authorization_codes.down.sql (1)
1-2: LGTM: Down migration is correct.The migration properly drops the index before the table and uses
IF EXISTSfor safety.internal/assets/migrations/000006_oidc_authorization_codes.up.sql (1)
1-10: LGTM: Authorization code table schema is well-designed.The table structure supports single-use enforcement with the
usedflag and includes an index onexpires_atfor efficient cleanup queries. The schema aligns with the model defined ininternal/model/oidc_authorization_code_model.go.internal/service/oidc_service.go (6)
240-248: LGTM: Client secret verification now uses bcrypt.This properly addresses the previous security concern about plaintext comparison. The use of
bcrypt.CompareHashAndPasswordprovides constant-time comparison and proper cryptographic hashing.
332-389: LGTM: Authorization code generation now includes replay protection.The database-backed storage (lines 338-349) properly addresses the previous critical security issue. Authorization codes are now tracked in the database, enabling single-use enforcement in
ValidateAuthorizationCode. The cleanup on JWT signing failure (lines 383-385) is good defensive programming.
391-484: LGTM: Authorization code validation implements proper replay protection.The implementation correctly:
- Verifies the code exists in the database (lines 418-426)
- Checks the
Usedflag to prevent replay attacks (lines 428-431)- Validates expiry (lines 433-436)
- Atomically marks the code as used (lines 456-460)
This fully addresses the previous critical security concern about replay attacks. The dual validation of
client_idandredirect_uri(both from JWT and DB) is defensive programming and doesn't hurt.
486-515: LGTM: PKCE validation correctly implements RFC 7636.The implementation properly supports both
S256(SHA256 hash) andplaincode challenge methods, with correct base64url encoding for S256.
728-733: LGTM: Client secrets are now hashed with bcrypt before storage.This properly addresses the previous security concern about storing plaintext secrets. The use of
bcrypt.GenerateFromPasswordwithDefaultCostprovides secure one-way hashing.
777-812: LGTM: Helper functions are straightforward and correct.The utility functions provide clean abstractions for scope manipulation, slice membership checks, and safe JWT claim extraction.
Security improvements:
1. HKDF key derivation:
- Replace raw sha256.Sum256() with proper HKDF (HMAC-based KDF)
- Uses domain-separated label 'oidc-aes-256-key-v1' for key derivation
- Applied to both encryptPrivateKey and decryptPrivateKey
- Provides better security properties than raw hash
2. Scope validation fix:
- Only add 'openid' scope if it's both requested AND in client's
allowedScopes
- Prevents bypassing client scope restrictions
- Respects configured allowedScopes
Both changes improve security posture while maintaining backward
compatibility.
The special case for adding 'openid' scope was redundant and could potentially bypass client scope restrictions. The main loop already correctly adds 'openid' to validScopes if it's in both requestedScopes and allowedScopes. Since 'openid' is already in the default scopes during client configuration (SyncClientsFromConfig), it will be available for clients that don't explicitly configure scopes. Clients can include or exclude 'openid' in their allowedScopes as needed. This ensures consistent enforcement of client scope restrictions with no special-case bypasses.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/service/oidc_service.go (2)
483-512: Consider deprecating PKCE "plain" method.While the PKCE implementation is correct, the "plain" method (lines 502-506) is discouraged by RFC 7636 which recommends S256 for security. Consider logging a warning when plain is used or only supporting S256.
🔎 Optional: Add warning for plain PKCE
case "plain": + log.Warn().Msg("PKCE 'plain' method is deprecated and insecure, consider using 'S256'") // Direct comparison if codeVerifier != codeChallenge { return errors.New("code_verifier does not match code_challenge")
638-645: Consider adding dynamic key identifiers for future key rotation.Line 641: The key ID is hardcoded as "default". For proper key rotation support, consider:
- Using a key fingerprint or database ID as the kid
- Adding kid to JWT headers when signing tokens
- Supporting multiple active keys in JWKS
This isn't critical for the current implementation but will be necessary if key rotation is implemented later.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/service/oidc_service.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/service/oidc_service.go (4)
internal/model/oidc_key_model.go (2)
OIDCKey(3-8)OIDCKey(10-12)internal/model/oidc_client_model.go (2)
OIDCClient(3-13)OIDCClient(15-17)internal/config/config.go (2)
UserContext(151-163)OIDCClientConfig(80-88)internal/model/oidc_authorization_code_model.go (2)
OIDCAuthorizationCode(3-10)OIDCAuthorizationCode(12-14)
🔇 Additional comments (7)
internal/service/oidc_service.go (7)
54-131: LGTM: Strong cryptographic implementation.The HKDF-based key derivation with domain separation and AES-GCM encryption provides robust protection for private keys. The backward compatibility handling for plaintext keys is well-designed.
133-236: LGTM: Robust key initialization with proper encryption.The Init method correctly handles RSA key loading/generation with encryption, deterministic key selection when multiple keys exist, and comprehensive error handling. The PKCS1/PKCS8 fallback parsing increases compatibility.
238-327: LGTM: Client validation methods are secure and correct.The client validation methods properly use bcrypt for secret comparison, correctly validate redirect URIs/grant types/response types, and enforce scope restrictions without bypassing client configurations.
388-481: LGTM: Robust authorization code validation with replay protection.The authorization code validation correctly implements replay protection by tracking usage in the database, validates JWT signatures and claims, and ensures client_id and redirect_uri consistency.
663-772: LGTM: Client synchronization correctly handles configuration-driven updates.The
SyncClientsFromConfigmethod properly handles client lifecycle with bcrypt-hashed secrets, JSON serialization of arrays, and appropriate defaults. The approach of always updating secrets from config (line 726-740) makes the configuration file the authoritative source, which is a reasonable design choice.
774-809: LGTM: Helper functions are correct and well-implemented.The helper functions properly handle edge cases (empty strings, whitespace, type safety) and provide clean abstractions for common operations.
329-386: No changes required. TheOIDCAuthorizationCodemodel does not have aDeletedAtfield, so it does not use GORM soft delete. TheDelete(&authCodeRecord)call on line 381 performs a hard delete, permanently removing the record as intended. The replay protection mechanism works correctly.Likely an incorrect or invalid review comment.
Access tokens include an 'aud' (audience) claim set to the client ID, but this was never validated during token validation. This allowed tokens issued for one client to be used by another client, violating the OAuth 2.0 security model. Changes: - Add ValidateAccessTokenForClient method that validates audience if expectedClientID is provided - Update ValidateAccessToken to call ValidateAccessTokenForClient (backward compatible, no audience check if not specified) - Update userinfo endpoint to accept optional client_id parameter and validate token audience matches it Security impact: - Prevents token reuse across different clients - Ensures tokens are scoped to specific clients as intended - Prevents attackers from using tokens issued for one client to access resources protected by another client
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/controller/oidc_controller.go (1)
480-489: Remove unused method to reduce code clutter.The
validateAccessTokenmethod is not called anywhere in the controller. TheuserinfoHandlerdirectly usescontroller.oidc.ValidateAccessTokenForClientinstead (line 353). Consider removing this unused helper method.🔎 Proposed cleanup
-// validateAccessToken validates an access token and extracts user context. -// Verifies the JWT signature using the OIDC service's public key, checks the -// issuer, and validates expiration. Returns the user context if valid, or an -// error if validation fails. -func (controller *OIDCController) validateAccessToken(accessToken string) (*config.UserContext, error) { - // Validate the JWT token using the OIDC service's public key - // This properly verifies the signature, issuer, and expiration - // Note: This method does not validate audience - use ValidateAccessTokenForClient for that - return controller.oidc.ValidateAccessToken(accessToken) -}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.review_triggerinternal/controller/oidc_controller.gointernal/service/oidc_service.go
✅ Files skipped from review due to trivial changes (1)
- .review_trigger
🧰 Additional context used
🧬 Code graph analysis (2)
internal/controller/oidc_controller.go (4)
internal/service/oidc_service.go (1)
OIDCService(41-46)internal/service/auth_service.go (1)
AuthService(39-46)internal/utils/app_utils.go (1)
GetContext(70-84)internal/config/config.go (1)
UserContext(151-163)
internal/service/oidc_service.go (6)
internal/model/oidc_key_model.go (2)
OIDCKey(3-8)OIDCKey(10-12)internal/model/oidc_client_model.go (2)
OIDCClient(3-13)OIDCClient(15-17)internal/config/config.go (3)
UserContext(151-163)Claims(100-106)OIDCClientConfig(80-88)internal/model/oidc_authorization_code_model.go (2)
OIDCAuthorizationCode(3-10)OIDCAuthorizationCode(12-14)internal/controller/context_controller.go (1)
Provider(40-44)internal/utils/security_utils.go (1)
GetSecret(13-28)
🔇 Additional comments (17)
internal/controller/oidc_controller.go (7)
1-61: LGTM! Clean controller structure and comprehensive OIDC endpoint setup.The controller is well-structured with proper dependency injection and all standard OIDC endpoints (discovery, authorize, token, userinfo, jwks) correctly registered.
63-87: LGTM! Compliant OIDC discovery document.The discovery handler correctly implements RFC 8414 with all required metadata and accurately advertises supported features.
89-204: LGTM! Authorization endpoint properly implements OAuth 2.0 authorization code flow with security fixes applied.The handler correctly validates all parameters before any redirects (preventing open redirect), includes PKCE support, preserves authentication state through login redirects, and enforces TOTP requirements.
206-329: LGTM! Token endpoint correctly implements authorization code exchange with PKCE validation.The handler properly validates all inputs, verifies client credentials via secure methods, validates PKCE code_verifier, and issues tokens following OIDC specifications.
331-372: LGTM! Userinfo endpoint with optional audience validation for backward compatibility.The handler correctly validates access tokens with optional audience checking when
client_idis provided. The optional nature maintains backward compatibility while allowing clients to opt into stronger validation by supplyingclient_idin the request.
374-388: LGTM! JWKS endpoint correctly delegates to service layer.Clean implementation with appropriate error handling.
390-478: LGTM! Helper functions implement secure credential handling.The credential extraction methods correctly support only
client_secret_basicandclient_secret_postauthentication methods, properly excluding query parameters for secrets to prevent exposure in logs and referrer headers.internal/service/oidc_service.go (10)
1-52: LGTM! Well-structured service foundation.Clean type definitions with appropriate dependencies including cryptographic libraries (bcrypt, hkdf) for secure key and secret management.
54-132: LGTM! Secure key encryption with proper KDF.The encryption implementation correctly uses HKDF for key derivation (addressing previous security concerns) with domain separation, and AES-256-GCM for authenticated encryption. Backward compatibility for plaintext keys is appropriately maintained.
134-237: LGTM! Robust key initialization with security improvements applied.The initialization correctly addresses previous concerns: deterministic key selection via
ORDER BY created_at DESC, warnings for multiple keys, and optional master key encryption with length validation.
239-328: LGTM! Client validation methods with security fixes applied.Client secret verification now uses bcrypt for constant-time comparison (preventing timing attacks), and scope validation correctly enforces client-configured scopes without special-case bypasses.
330-482: LGTM! Authorization code flow with comprehensive replay protection.Critical security improvements have been applied: authorization codes are now tracked in the database with single-use enforcement, PKCE parameters are properly stored and validated, and all security checks (expiration, client/redirect URI matching) are correctly implemented.
484-513: LGTM! PKCE validation correctly implements RFC 7636.The method properly supports both S256 and plain challenge methods with correct cryptographic operations.
515-608: LGTM! Access token validation with audience checking for cross-client protection.The new
ValidateAccessTokenForClientmethod correctly implements optional audience validation to prevent token reuse across clients. The design maintains backward compatibility by making audience checking opt-in via theexpectedClientIDparameter.
610-663: LGTM! ID token generation and RFC 7517-compliant JWKS.ID tokens include all required OIDC claims, and the JWKS encoding correctly uses minimal-octet representation for the RSA exponent (addressing previous RFC compliance concerns).
665-785: LGTM! Client synchronization with secure secret hashing.The client sync correctly hashes secrets with bcrypt before storage (addressing previous security concerns), applies sensible defaults, and properly handles both new client creation and existing client updates.
787-822: LGTM! Clean utility functions.The helper methods provide straightforward string and slice operations with appropriate edge case handling.
|
As far as I can tell, all relevant actions have been taken to add OIDC provider capability in a secure manner to tinyauth. Even if this particular approach isn't deemed suitable, I hope the capability makes it into tinyauth, as this would be a killer feature in such a small package. |
|
@shreknel thank you so much for this pull request. I will have to go through the spec myself to ensure that everything is implemented correctly before merging. Would you mind if I took over the pull request? I immediately see some things I would like to do differently and I believe that doing it through conversations would take a significant amount of time. |
|
@steveiliop56 I don't mind at all. Cheers! |
|
Alright, closing in favor of #550. |
2 participants
This commit adds OpenID Connect (OIDC) provider functionality to tinyauth, allowing it to act as an OIDC identity provider for other applications.
Features:
Validation:
Configuration:
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.